Add investment parsing support for 14 BDC tickers - #990
Conversation
…, PSBD, RWAY, LIEN, PNNT, BCIC, GECC, OFS, TRIN
dgunning
left a comment
There was a problem hiding this comment.
Thanks for this — and for coming back to the same subsystem after #747. The BDC label formats are a genuinely nasty surface and the breadth here is welcome.
I checked it by running the parser over two BDCs the PR doesn't mention, on main and on this branch, and diffing the (identifier, company_name, investment_type) triples. Good news first, then one blocker.
The classification gain is real
| investments | Unclassified → typed |
typed → Unclassified |
|
|---|---|---|---|
| MAIN | 751 | 120 | 0 |
| OBDC | 640 | 11 | 0 |
Nothing regresses in the direction of losing a type. That's a solid improvement on issuers you weren't targeting.
Blocker: company_name regresses on 13 investments
Same run, company_name changes on 13 investments across those two BDCs — and each change is wrong. Your 157 tests pass because none of them cover MAIN or OBDC labels.
Pattern 1 — the industry is taken as the company (12 of 13, all OBDC). On labels shaped <Company> | <Industry> <type> investment | <relationship>:
label: Wingspire Capital Holdings LLC | Specialty finance equity investment | Affiliated
main: company='Wingspire Capital Holdings LLC' type='Unclassified'
PR: company='Specialty finance' type='equity'
The type does get classified, which is the win — but the company name is replaced by the industry. Also hits AAM Series 1.1 Rail and Domestic Intermodal Feeder, LLC, AAM Series 2.1 Aviation Feeder, LLC, Amergin Asset Management, LLC, Blue Owl Cross-Strategy Opportunities LLC, Fifth Season Investments LLC, LSI Financing 1 DAC, LSI Financing LLC and Wingspire Capital Holdings LLC (several appear twice under ... 1 and ... | Affiliated variants).
A related one on the same BDC, where a leading relationship fragment gets eaten:
label: Controlled/affiliated - debt commitments, First lien senior secured revolving loan
main: company='Controlled/affiliated - debt commitments'
PR: company='debt commitments'
Pattern 2 — a share-class prefix is taken as the company (1, MAIN).
label: DTE Enterprises, LLC | Class AA Preferred Member Units (non-voting)
main: company='DTE Enterprises, LLC'
PR: company='Class AA'
That one looks connected to the new 'Class AA units' entry in INVESTMENT_TYPES matching inside Class AA Preferred Member Units.
This is the failure mode we treat as most serious: no exception, no empty field, just a confidently wrong company name flowing into PortfolioInvestment.company_name.
Reproducing
from edgar.bdc import find_bdc
inv = find_bdc("OBDC")[0].portfolio_investments()
print({i.company_name for i in inv if "Specialty finance" in i.identifier})
# main: the eight real company names
# this branch: {'Specialty finance'}One decision to confirm, not necessarily blocking
525 of MAIN's 751 investment types change, mostly dropping a tranche suffix — 'Secured Debt 1' → 'Secured Debt', 'First lien senior secured revolving loan 1' → '... revolving loan'. I think that's the right normalization, since identifier still disambiguates the tranches, but it silently changes results for anyone grouping by investment_type. Please confirm it's deliberate; if so it wants a CHANGELOG note under [Unreleased].
What would land this
- Preserve the company span on the two label shapes above.
- Add MAIN and OBDC labels to
tests/test_bdc.py. The current 157 cases are organised per new ticker, so a cross-issuer regression has nowhere to fail — the 13 above went unnoticed for exactly that reason. A handful of parametrized cases from already-supported issuers would close the gap. - Confirm the tranche-suffix normalization.
Everything else looks good: no ticker symbols hardcoded in the parser, no existing test modified or deleted, and all 228 tests in tests/test_bdc.py pass on the branch. It's the cross-issuer blast radius that needs containing, not the approach.
🤖 Review assisted by Claude Code
|
Thanks, fixed in the latest push. Preserved the company span for the OBDC pipe-delimited and comma-delimited label shapes. All 161 investment identifier parser tests pass. |
The descriptor-pipe branch added in b5109e9 matches case-insensitively and returned the captured span verbatim, so the label's own casing became part of the value. OBDC writes "Specialty finance equity investment", which yielded 'equity' and 'debt' — the only lowercase-initial types across MAIN and OBDC, sitting next to 'Preferred Equity', 'Common Equity' and 'Secured Debt' from every other branch. Grouping by investment_type split the same concept across two buckets, which is the property this PR's CHANGELOG entry sells. Title-cased at the one site that can produce it. Measured over MAIN (751 investments) and OBDC (640): 13 values recased, no company_name touched, no type gained or lost. Both remain at zero regressions against the merge-base. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dgunning
left a comment
There was a problem hiding this comment.
Approving. The blocker is fixed, and I verified it the same way I found it — parsed MAIN and OBDC on this branch and on the merge-base (2df53a2c, not current main, so the diff isolates the PR) and compared the (identifier, company_name, investment_type) triples.
The 13 wrong company names are gone
| investments | Unclassified → typed |
typed → Unclassified |
company_name changed |
|
|---|---|---|---|---|
| MAIN | 751 | 120 | 0 | 0 |
| OBDC | 640 | 13 | 0 | 0 |
The classification gain is intact and OBDC's went 11 → 13. Your reproduction case now returns the real names:
['AAM Series 1.1 Rail and Domestic Intermodal Feeder, LLC',
'AAM Series 2.1 Aviation Feeder, LLC', 'ABF - Specialty finance',
'Amergin Asset Management, LLC', 'Blue Owl Cross-Strategy Opportunities LLC',
'Fifth Season Investments LLC', 'LSI Financing 1 DAC', 'LSI Financing LLC',
'Wingspire Capital Holdings LLC']
The DTE Enterprises, LLC / Class AA case is fixed by the right mechanism too — adding 'Class AA Preferred Member Units' to INVESTMENT_TYPES so the longer type wins the match, rather than special-casing the label.
Tranche normalization is confirmed deliberate and documented in [Unreleased], and the measured split matches the description: MAIN 405 suffix drops + 120 classification gains, OBDC 62 + 13, no unexplained churn.
One thing I fixed rather than sent back
7dbe51f6, pushed to this branch. The new descriptor-pipe branch matches case-insensitively and returned the captured span verbatim, so the filer's typography became the value: OBDC writes "Specialty finance equity investment", which produced 'equity' (11) and 'debt' (2) — the only lowercase-initial types across either BDC, sitting beside 'Preferred Equity', 'Common Equity' and 'Secured Debt' from every other branch. Grouping by investment_type then split the concept in two, which is the property your CHANGELOG entry sells. Title-cased at the single site that can produce it, with a parametrized test over three label casings.
Re-measured after that change: 13 values recased, no company_name touched, no type gained or lost, no lowercase-initial types left. tests/test_bdc.py is 236 passing.
One residual, not blocking
The four new parametrized cases cover exactly the four labels I reported. That closes those, but the original problem was that 157 tests passed while nothing covered MAIN or OBDC at all — four labels is not coverage of those issuers, so the next cross-issuer regression still has nowhere to fail. Worth a follow-up that pins a slice of real MAIN/OBDC triples, rather than growing this PR.
Thanks for the careful turnaround on this one — the diagnosis and the fixes were both right.
🤖 Review assisted by Claude Code
* docs(changelog): the BDC entry documents the next release, not a published one #990 branched from 2df53a2 on 2026-08-07, when the entries now dated 5.46.0 were still under [Unreleased]. It added its own entry to that block, and eight days later the squash merge applied the hunk by context — into a section that had since been released. So 5.46.0's published changelog claimed a feature it does not contain, and the BDC work would have shipped in the next release undocumented. Moved to [Unreleased] under a `### Changed` heading, with the two label-shape fixes and the ticker count added, since those are what a reader upgrading actually needs. 5.46.0 keeps its other 22 entries unchanged. Worth knowing for any long-lived branch: a CHANGELOG hunk written against [Unreleased] does not follow that heading when a release is cut underneath it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): 5.49.0 Seven entries, one Changed and six Fixed. The headline is section extraction: a 10-K written with colon-separated item headers went from one section to fifteen, and Item 1C (Cybersecurity, mandatory since December 2023) and Item 16 now resolve on filings whose table of contents omits them — which was most modern 10-Ks. Across the parity corpus the new parser moved from level with the deprecated one to ahead of it on 10-K, +0.1% to +3.4%. Also folds in the BDC changelog entry that #990 landed under the already published 5.46.0 section: its branch predated that release, so the squash merge placed the hunk by context into a section that had since been dated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…9.0 section The branch was written when its bullet sat under [Unreleased]. 5.49.0 shipped in the meantime, which put a version heading above those bullets, so merging main placed this entry at the tail of an already-published section — cleanly, with no conflict to notice. Moved under a fresh [Unreleased] / Fixed. This is the second time this trap has fired (it put dgunning#990's entry inside 5.46.0), and both times git merged without complaint: the surrounding bullets are stable context, so only the heading above them moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… host (#989) * fix(cache): cache rules never matched data.sec.gov (+ add companyfacts) `_get_cache_rules()` builds its single top-level key from `SEC_BASE_URL` (`www.sec.gov`), but httpxthrottlecache matches the request HOST against that key before it ever looks at the path. `/submissions` and `/api/xbrl/companyfacts` are served from `SEC_DATA_URL` (`data.sec.gov`), so: re.match(r'.*www\.sec\.gov', 'data.sec.gov') -> None No rule under that key has ever applied to a data.sec.gov request. Verified live: a fresh process logs `No patterns matched data.sec.gov` and goes to the network every time. This means the `/submissions` TTL has never taken effect -- including the deliberate 600s -> 30s reduction in 655f5d3 for Issue #471. It also means `get_facts()` (companyfacts) re-downloads on every call in every process. Fix: build one key per host actually used by `edgar.urls` and file each rule under the host that really serves it. A custom mirror that points EDGAR_BASE_URL and EDGAR_DATA_URL at the same host merges into a single key, so mirror support is preserved. Also adds the missing `/api/xbrl/companyfacts/.*` rule under the data host, with the same 30s budget as `/submissions`: both are invalidated by the same event (a new filing lands), so a longer companyfacts-specific TTL would reintroduce exactly the staleness Issue #471 removed. Tests: `tests/issues/regression/test_data_sec_gov_cache_domain.py`, 5 cases. 3 of them fail against the pre-fix code (they pin the regression itself, not just the fixed behaviour); the other 2 are negative guards for www.sec.gov and an unrelated host, unaffected either way. CHANGELOG.md: entry under [Unreleased] / Fixed, per CONTRIBUTING.md. * fix(cache): key cache rules by exact host, built by the matcher's own parser The previous commit fixed data.sec.gov by adding a second key, but kept the hand-built `.*<domain>` pattern. Two holes survived, both raised by @paultiq pointing at GH #490 as the origin: Unanchored keys leak across hosts. `.*mirror.example.org` also matches `data.mirror.example.org`, and `controller.get_rules` returns the FIRST key that matches, so a mirror configured as base=mirror.example.org / data=data.mirror.example.org resolved its submissions and companyfacts requests to the base rule set — which has no rule for them. That is this same bug, moved from sec.gov to mirrors. Hand-parsing disagrees with the matcher. The key is compared against `request.url.host`; `https?://([^/]+)` keeps the case, port, `user@` and percent-encoding that httpx normalises away, so a capitalised host, a port or credentials in EDGAR_DATA_URL produced a key no real request can match. `_host_key` now takes the host from `httpx.URL(...).host` — the same parser that produces it at match time — and matches it exactly, so the two sides cannot disagree and a rule written for one host cannot answer for another. A URL with no host is logged and matches nothing: a misconfigured mirror goes uncached rather than borrowing another host's rules. Also pins httpxthrottlecache>=0.6.1, whose clamp for the clock-skew crash (paultiq/httpxthrottlecache#43) is what caching data.sec.gov now reaches. Tests: 7 of the 12 fail on the previous commit — the three cross-host cases (including a lookalike host that inherited the cache-forever Archives rule) and the four parser-mismatch cases. Two of them only fail after fixing the test helper itself: it fell through to later keys when the real matcher stops at the first, which made every cross-host assertion unable to fail. * test(cache): pin the replica against the real matcher, split multi-rule asserts The file checks _get_cache_rules through _rule_for, a replica of httpxthrottlecache's matcher. A replica can be wrong in the same direction as the code it checks, and this one already was once — so it is now pinned against controller.get_rule_for_request itself, over deliberately overlapping keys, which is the only shape where a fall-through replica and the library disagree. Agreement on the live (non-overlapping) rules is free and could not fail alone. Also: one case per rule instead of four asserts in one test, so a break names the rule it broke; /Archives cache-forever asserted by identity, apart from the TTL cases; and the percent-encoded mirror host from the PR table now has the test the other three URL forms already had. Verified: 23 pass, 8 fail against the previous commit's httpclient.py, all 23 pass in isolation and in reverse order. * test(cache): name the PR the regression test guards `scripts/check_regression_provenance.py` refuses a regression test whose module docstring does not link the issue, PR or bead it guards, and the fast job runs it before the tests — so the whole job failed in 65s without running a single test. The docstring already explained the bug at length; what the gate wants is the one canonical link, which here is the PR itself. * docs(changelog): move the #989 entry out of the published 5.49.0 section The branch was written when its bullet sat under [Unreleased]. 5.49.0 shipped in the meantime, which put a version heading above those bullets, so merging main placed this entry at the tail of an already-published section — cleanly, with no conflict to notice. Moved under a fresh [Unreleased] / Fixed. This is the second time this trap has fired (it put #990's entry inside 5.46.0), and both times git merged without complaint: the surrounding bullets are stable context, so only the heading above them moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dwight Gunning <dgunning@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds structured investment identifier parsing for TSLX, MSDL, GSBD, MFIC, BCSF, PFLT, PSBD, RWAY, LIEN, PNNT, BCIC, GECC, OFS, and TRIN.
Includes regression coverage for company names, investment types, XBRL field anchors, aliases, facilities, series, and rollups. Parser tests: 157 passed.